Skip to content

[API-95] Add resolve endpoint, improve permalink matching#55

Merged
raymondjacobson merged 3 commits intomainfrom
rj-api-95
Apr 29, 2025
Merged

[API-95] Add resolve endpoint, improve permalink matching#55
raymondjacobson merged 3 commits intomainfrom
rj-api-95

Conversation

Comment thread api/v1_tracks.go Outdated
Comment thread api/v1_tracks.go
Comment thread api/v1_tracks.go Outdated
splits := strings.Split(permalinks[i], "/")
if len(splits) != 2 {
if match := trackURLRegex.FindStringSubmatch(permalink); match != nil {
handles[i] = strings.ToLower(match[1])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart, though iirc the query also does toLower - probably good to be defensive

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the query doesn't, ill update it to though

Copy link
Copy Markdown
Contributor

@stereosteve stereosteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Few notes.

SELECT user_id FROM users
WHERE
handle_lc = lower(@handle)
AND is_current = true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we still is_current?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont

Comment thread api/server.go
err := app.pool.QueryRow(context.Background(), sql, handle).Scan(&userId)
app.resolveHandleCache.Set(handle, userId)
return userId, err
user_id, err := app.queries.GetUserForHandle(context.Background(), handle)
Copy link
Copy Markdown
Contributor

@stereosteve stereosteve Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should actually if err != nil {return err} here. My bad on that.

This should return pgx.ErrNoRows if not found I believe.

So returning on error avoids setting no_exist_handle = 0 in the cache... which would be bad if the indexer is behind and it might work on next try.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah great catch

Comment thread api/v1_users_handle.go Outdated
return fiber.NewError(fiber.StatusBadRequest, "Missing handle parameter")
}

userId, err := app.queries.GetUserForHandle(c.Context(), handle)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could app. resolveUserHandleToId here to use cache.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file isnt' even needed. oops

@raymondjacobson raymondjacobson merged commit 0552250 into main Apr 29, 2025
2 checks passed
@raymondjacobson raymondjacobson deleted the rj-api-95 branch April 29, 2025 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants